Canvas fit: decouple logical from physical dimensions (closes #17) - #63
Conversation
…pdate sandboxes, flake.nix cleanup, add .envrc (closes #17)
|
@dabd I haven't looked at the code properly (I can look at this properly on Monday if you like), but I have run the tyrian version. At the moment the result is that the output is stretched or squashed somehow, where presumably what we want is to redraw it cleanly at the requested size, no? |
That is right, currently I am scaling X and Y independently to match the canvas. |
bdfa79e to
ef935f7
Compare
…esize, keep renderToFit for non-uniform fill, flake.nix add git to dev shell
ef935f7 to
71e1bcd
Compare
|
@davesmith00047 Pushed an improvement in 71e1bcd and created issue #64 to solve a separate problem that is apparent in this PR: charts in small canvases can show overlapping legends. |
| let | ||
| pkgs = import nixpkgs { inherit system; }; | ||
| jdkToUse = pkgs.jdk17; | ||
| sbtWithJRE = pkgs.sbt.override { jre = jdkToUse; }; |
There was a problem hiding this comment.
removed sbt since we are not using it
| pkgs.git | ||
| pkgs.nodejs | ||
| pkgs.yarn | ||
| pkgs.nodePackages_latest.http-server |
There was a problem hiding this comment.
removed unused package
|
Looks good! |
davesmith00047
left a comment
davesmith00047
left a comment
There was a problem hiding this comment.
Looking good! I have just one small request around the render method naming / docs.
|
|
||
| extension [C <: Chart, S <: ChartStyle](styled: StyledChart[C, S]) | ||
|
|
||
| def renderToFit(ctx: CanvasRenderingContext2D)(using |
There was a problem hiding this comment.
Can you add scaladoc's to these? It isn't clear to me from the naming, or a glance at the code, what these different rendering modes do.
There was a problem hiding this comment.
Thanks! I'm still not sure what the use case is for renderFillCanvas, but it's doing no harm, no reason not to live with it for a while and see how it pans out. 🙂
There was a problem hiding this comment.
Good point, let's keep it lean.
The intention was for renderFillCanvas to scale the already laid-out drawing independently in X and Y to fill the canvas (but this could warp elements). Can't think of a very compelling use case 🤔 , - maybe draw small things like spark lines, or lots of thumbnails quickly.
renderAtCanvasSize recomputes layout for the new size, avoiding this warping, so I will just keep this.
…asWith for non-uniform fill, renderAtCanvasSize and renderAtCanvasSizeWith for redraw
2 participants
style width/height to the canvas size.
Testing
renderToFit, chart fills canvas without cropping; withrenderTo, it crops or doesn’t fill.switch renderToFit to renderTo in sandbox-tyrian/src/florence/sandbox/CustomChart.scala.
Known limitations
Legend overlap on small canvases:
renderToResizeredraws without distortion but does not auto-reserve space for the legend. On short canvases (e.g., 600x200 shown below) the legend can overlap the plot.
Temporary workarounds: move legend to Top/Bottom, reduce legend font/symbol size, or increase margins.
See issue #64 for details.
600x200:
1300x300:
